-
Notifications
You must be signed in to change notification settings - Fork 8.5k
[Discover] Fix truncation for longer text in table #241440
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
…/kibana into discover-fix-one-line-truncation
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Async chunks
History
|
| '.euiDataGridRowCell__content--defaultHeight > div': { | ||
| maxInlineSize: '100%', | ||
| overflow: 'hidden', | ||
| textOverflow: 'ellipsis', | ||
| whiteSpace: 'nowrap', | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like the root cause of the issue may be that we wrap cell contents in a div which defaults to display: block, preventing the EUI styles from being inherited correctly. Instead of redefining the EUI styles here, what if we just change the wrapping element to a span in InTableSearchHighlightsWrapper so the styles are inherited correctly?
Line 64 in faf0c28
| return <div ref={cellValueRef}>{children}</div>; |
I gave it a quick shot locally and it seemed to work as expected.
Summary
Closes #238831
After wrapping cell into a
divin purpose of in-table-search (we need arefthere) we lost cell content truncation, because styles applied by EUI are not inherited further.UI before in-table-search

UI after in-table-search

EUI docs

As a solution, I firstly added a
.eui-textTruncateclass directly to that new div, however it was then applied to multilines as well and the class forced one-line text render, no matter of body cell lines count.Eventually I've targeted
.euiDataGridRowCell__content--defaultHeight > div(targeting --defaultHeight is already a workaround for one line summary truncation) and applied truncation styles manually. Another possibility would be applyingeuiTextTruncate()helper, however it doesn't work in an object syntax we use here (following to Emotion standards).I'm open to refactor to css`` syntax, which accepts
euiTextTruncate()if we prefer to use EUI helper, instead of keeping Emotion syntax aligned.Checklist
Check the PR satisfies following conditions.
Reviewers should verify this PR satisfies this list as well.
release_note:breakinglabel should be applied in these situations.release_note:*label is applied per the guidelinesbackport:*labels.Identify risks
Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss.
Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging.